Skip to content

Trim AlgorithmsInterfaceExtensions to a minimal NestedAlgorithm#115

Merged
mtfishman merged 16 commits into
mainfrom
mf/nested-algorithm
May 20, 2026
Merged

Trim AlgorithmsInterfaceExtensions to a minimal NestedAlgorithm#115
mtfishman merged 16 commits into
mainfrom
mf/nested-algorithm

Conversation

@mtfishman
Copy link
Copy Markdown
Member

@mtfishman mtfishman commented May 19, 2026

Trim AlgorithmsInterfaceExtensions (AIE) to a focused set of helpers built on top of AlgorithmsInterface:

  • NestedAlgorithm — abstract AI.Algorithm whose step! delegates to a subsolve via initialize_subsolve / finalize_substate!.
  • NestedState — abstract AI.State that wraps an inner substate and forwards :iterate accesses to it, so the iterate is shared across nesting levels without duplication.
  • StopWhenConverged + iterate_diff — convergence-based stopping criterion plus the per-iterate-type hook it dispatches on.
  • AbstractAlgorithm + select_algorithm / default_algorithm — MatrixAlgebraKit-style algorithm-selection helpers. Selection-relevant inputs are packed into an args tuple so the value and type domains stay disjoint, and operations register their default by overloading default_algorithm(::typeof(f), ::Type{<:Tuple}).

The belief-propagation code is refactored into two AlgorithmsInterface Problem / Algorithm / State triples:

  • BeliefPropagation{Problem, Algorithm, State} — outer iteration. The algorithm is an AIE.NestedAlgorithm carrying a single inner subalgorithm; the state is an AIE.NestedState wrapping the inner substate so the message store persists across sweeps.
  • BeliefPropagationSweep{Problem, Algorithm, State} — one sweep over edges. A plain AI.Algorithm whose step! performs one message update by delegating to a MessageUpdateAlgorithm strategy.

A MessageUpdateAlgorithm is a lightweight strategy supertype (not an AI.Algorithm): message_update!(algorithm, cache, factors, edge) is the single per-edge entry point. SimpleMessageUpdate is the default, carrying normalize and contraction_alg. Iteration- or edge-dependent behavior is added by defining a new strategy subtype and overloading message_update! (for per-edge variation) or a new sweep algorithm subtype and overloading AIE.initialize_subsolve (for per-iteration variation).

The top-level beliefpropagation(factors, messages; …) entry point exposes stopping_criterion and message_update_algorithm kwargs, each accepting either an explicit instance, a NamedTuple forwarded to a default constructor, or flat kwargs. stopping_criterion = (; maxiter = 10, tol = 1.0e-10) combines a StopAfterIteration and a StopWhenConverged via |.

…ubstate!`

Rename the `NestedAlgorithm` step hooks from `get_subproblem` /
`set_substate!` to `initialize_subsolve` / `finalize_substate!`, and move
the indexed-list dispatch (`algorithm.algorithms[state.iteration]`) out
of the abstract type's default into `DefaultNestedAlgorithm`'s own
`initialize_subsolve` method. The abstract `NestedAlgorithm` now has no
default `initialize_subsolve` impl (throws `MethodError`), so subtypes
must provide their own — the previous default silently assumed every
subtype carried an `algorithms` vector, which is too narrow.

`BeliefPropagation` and `BeliefPropagationSweep` get an explicit
`AIE.initialize_subsolve` override mirroring the indexed-list shape;
the existing `AIE.set_substate!` override on `BeliefPropagationSweep`
is renamed to `AIE.finalize_substate!`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mtfishman added a commit that referenced this pull request May 19, 2026
Sync the apply-PR's local `NestedAlgorithm` definition with the rename
landing in #115. Once #115 merges, this local definition will be
removed entirely in favor of `AIE.NestedAlgorithm`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

❌ Patch coverage is 77.94118% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.30%. Comparing base (bd91366) to head (5d56f9f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...terfaceExtensions/AlgorithmsInterfaceExtensions.jl 66.07% 19 Missing ⚠️
src/beliefpropagation/beliefpropagation.jl 86.25% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #115      +/-   ##
==========================================
- Coverage   75.81%   73.30%   -2.52%     
==========================================
  Files          23       21       -2     
  Lines         980      929      -51     
==========================================
- Hits          743      681      -62     
- Misses        237      248      +11     
Flag Coverage Δ
docs 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Delete from AIE:
- `DefaultNestedAlgorithm` (struct, constructor, and its
  `initialize_subsolve` indexed-list impl).
- `nested_algorithm` factory function + `max_iterations`.
- All `FlattenedAlgorithm` machinery (struct, state, helper).
- `AlgorithmIterator` / `DefaultAlgorithmIterator` /
  `algorithm_iterator`.
- `with_algorithmlogger`.
- `NonIterativeAlgorithm` / `DefaultNonIterativeAlgorithmState`.

Keep the minimum BP / nested-solve still needs: `Problem`, `Algorithm`,
`State`, `DefaultState`, `AI.initialize_state` / `initialize_state!` /
`increment!`, and the minimal `NestedAlgorithm` (abstract type +
`initialize_subsolve` + `finalize_substate!` + `AI.step!`). Future
features can be added back as concrete subtypes when actually needed.

Delete the placeholder sweep / DMRG scaffolding (`src/sweeping/`,
`test/test_dmrg.jl`, `test/test_sweeping.jl`): `EigsolveRegion` /
`EigenProblem` / `dmrg` / `select_algorithm` were not actually wired up
(the `solve!` body threw `not implemented yet`), and the tests covered
only construction shape. Inline the kwarg-expansion helpers
(`extend_columns`, `rows`, ...) that lived in `sweeping/utils.jl` into
`beliefpropagation/beliefpropagationproblem.jl` — its only remaining
consumer.

Prune `test/test_algorithmsinterfaceextensions.jl` to the surface that
still exists; add a `TestNestedAlgorithm` concrete subtype mirroring how
`BeliefPropagation` shapes itself on top of the new minimal interface,
and a test that the bare `initialize_subsolve` default throws.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mtfishman mtfishman changed the title Refactor NestedAlgorithm hooks: initialize_subsolve + finalize_substate! Trim AlgorithmsInterfaceExtensions to a minimal NestedAlgorithm May 19, 2026
mtfishman and others added 4 commits May 19, 2026 16:19
`AlgorithmsInterfaceExtensions` is now just the `NestedAlgorithm`
abstract type plus `initialize_subsolve` / `finalize_substate!` /
`AI.step!`. The `Problem` / `Algorithm` / `State` abstract types and
`DefaultState` are gone, along with the `AI.initialize_state` /
`initialize_state!` / `increment!` overloads that hung off them.

Belief propagation grows its own state machinery instead of leaning on
AIE: a `BeliefPropagationState <: AI.State` (mutable, `iterate` /
`iteration` / `stopping_criterion_state`), plus per-algorithm
`AI.initialize_state` / `initialize_state!` / `increment!` overloads on
`Union{BeliefPropagation, BeliefPropagationSweep}`. This mirrors the
pattern `BPApplyGate` already uses in the apply-operator path — each
algorithm owns its state, no shared default.

`BeliefPropagationProblem`, `BeliefPropagation`, and
`BeliefPropagationSweep` now subtype the bare `AI.Problem` /
`AI.Algorithm` types; the `StopWhenConverged` dispatches accept
`AI.Problem` / `AI.Algorithm` / `AI.State` since `StopWhenConverged`
itself is the unique type doing the disambiguation.

`test_algorithmsinterfaceextensions.jl` is rewritten to define its
own `TestProblem` / `TestChildAlgorithm` / `TestChildState` /
`TestNestedAlgorithm` directly on `AI.*`, so the test exercises the
same "each algorithm owns its state" pattern.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Layer 1 — outer BP loop:
  `BeliefPropagationProblem` / `BeliefPropagationAlgorithm` /
  `BeliefPropagationState` (iterative, `<: AIE.NestedAlgorithm`)

Layer 2 — one sweep over edges:
  `BeliefPropagationSweepProblem` / `BeliefPropagationSweepAlgorithm` /
  `BeliefPropagationSweepState` (iterative, `<: AIE.NestedAlgorithm`)

Layer 3 — single-edge message update:
  `MessageUpdateProblem` / `SimpleMessageUpdateAlgorithm` /
  `MessageUpdateState` (non-iterative, overrides `AI.solve_loop!` —
  same shape `BPApplyGate` uses in the gate-apply code)

Every layer subtypes `AI.Problem` / `AI.Algorithm` / `AI.State`
directly. No `Union{...}` shortcuts — each layer's `initialize_state` /
`initialize_state!` / `initialize_subsolve` is its own method. The
`StopWhenConverged` dispatches accept `AI.Problem` / `AI.Algorithm` /
`AI.State` since `StopWhenConverged` itself is the unique
disambiguating type.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
API first, then the three layers from outer (BP) to inner (message
update), then the supporting `StopWhenConverged` stopping criterion +
`iterate_diff` helper, then the low-level kwarg utilities at the bottom.
Reads as "what's the API → how it's composed → supporting pieces" the
way the file is most likely to be skimmed.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
`StopWhenConverged` is fully generic — its dispatches accept
`AI.Problem` / `AI.Algorithm` / `AI.State`, with `StopWhenConverged`
itself as the unique disambiguating type. The only piece that needed
to live in BP was the metric: the BP `iterate_diff(::MessageCache,
::MessageCache)` override.

AIE now owns: `StopWhenConverged`, `StopWhenConvergedState`, the four
`AI.*` overloads, and a bare `iterate_diff(a, b)` verb that throws by
default. BP provides its concrete `AIE.iterate_diff(::MessageCache,
::MessageCache)`. The top-level `ITensorNetworksNext` namespace
re-exports `StopWhenConverged` + `iterate_diff` for callers that use
the package-qualified names.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment thread src/beliefpropagation/beliefpropagationproblem.jl Outdated
Comment thread src/beliefpropagation/beliefpropagationproblem.jl Outdated
Comment thread src/beliefpropagation/beliefpropagation.jl Outdated
…teProblem`

The edge is per-step data; it belongs on the problem side, not the
algorithm side. After the move:

- `MessageUpdateProblem{Factors, Edge <: AbstractEdge}` carries
  `factors` + `edge` (type parameter renamed from `E` to `Edge`).
- `SimpleMessageUpdateAlgorithm{ContractionAlg}` is now just
  `normalize` + `contraction_alg` — no per-edge data.
- `BeliefPropagationSweepProblem` gains an `edges` field; its
  `initialize_subsolve` picks `edge = problem.edges[state.iteration]`
  and threads it into the `MessageUpdateProblem`.
- `BeliefPropagationProblem` gains an `edges` field too, and the outer
  `initialize_subsolve` threads it down to the sweep subproblem.
- The entry-point `do edge` closure becomes `do _` since the algorithm
  no longer needs the edge.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@jack-dunham
Copy link
Copy Markdown
Contributor

jack-dunham commented May 19, 2026

I was thinking BeliefPropagationSweepAlgorithm could host an ordered mapping from an edge to a message update algorithm. Then one does:

function initialize_subsolve!(problem::BeliefPropagationProblem, algorithm::BeliefPropagationAlgorithm, state::BeliefPropagationState)
    subalgorithm = algorithm.sweeps[state.iteration] # a BeliefPropagationSweepAlgorithm
    edges = keys(subalgorithm.updates)
    subproblem = BeliefPropagationSweepProblem(problem.factors, edges)
    substate = initialize_state(subproblem, subalgorithm; iterate = state.iterate)
    return subproblem, subalgorithm, substate
end

and also (property/variable names for illustration only)

function initialize_subsolve!(problem::BeliefPropagationSweepProblem, algorithm::BeliefPropagationSweepAlgorithm, state::BeliefPropagationSweepState)
    edge = problem.edges[state.iteration]
    subalgorithm = algorithm.updates[edge]
    subproblem = MessageUpdateProblem(problem.factors, edge)
    substate = initialize_state(subproblem, subalgorithm; iterate = state.iterate)
    return subproblem, subalgorithm, substate
end

mtfishman and others added 2 commits May 19, 2026 17:46
…gationSweepAlgorithm`

At the outer BP layer the edge update order is an algorithmic choice
(which edges to sweep and in what order, potentially varying per rep),
not problem data. So edges now live on `BeliefPropagationSweepAlgorithm`
— one sweep algorithm is "do a sweep with these edges". The outer
`initialize_subsolve` transfers them into a `BeliefPropagationSweepProblem`
when stepping down a layer.

Side effect: `BeliefPropagationSweepAlgorithm` no longer needs an
`algorithms::Vector` field. The per-edge `SimpleMessageUpdateAlgorithm`s
were always identical copies (they no longer carry an edge), so it
collapses to a single `message_update_algorithm` template — matching the
shape `ApplyOperators` uses in the gate-apply code.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Edges shouldn't be stored at both the outer BP algorithm level (as the
algorithmic edge-ordering choice) and the sweep algorithm level (as
runtime data that gets copied into the sweep problem). Keep them only
on `BeliefPropagationAlgorithm`; the outer `initialize_subsolve` then
transfers them into the `BeliefPropagationSweepProblem` when stepping
down.

`BeliefPropagationSweepAlgorithm` is now just `message_update_algorithm`
+ `stopping_criterion` (the stopping criterion is sized to the edges at
construction time in `beliefpropagation()`).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mtfishman
Copy link
Copy Markdown
Member Author

I was thinking BeliefPropagationSweepAlgorithm could host an ordered mapping from an edge to a message update algorithm.

Interesting idea, I'll try to incorporate that into the PR.

mtfishman and others added 7 commits May 19, 2026 18:54
Make `BeliefPropagationSweepAlgorithm.algorithms` indexable by edge
(default: a `Dict{Edge, MessageUpdateAlgorithm}` populated with the same
template at construction time), and drop the
`Algorithms <: AbstractVector{ChildAlgorithm}` constraint from both
`BeliefPropagationAlgorithm` and `BeliefPropagationSweepAlgorithm` so any
indexable container works.

Edges remain sourced from `BeliefPropagationSweepProblem` (the source of
truth); the sweep `initialize_subsolve` looks up
`algorithm.algorithms[edge]` to pick the per-edge update algorithm.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The file now defines three problem/algorithm/state triples plus the
top-level `beliefpropagation` entry point, so the old name is misleading.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Public-API changes:

  * `beliefpropagation` no longer takes a `maxiter` kwarg; pass either
    `stopping_criterion = (; maxiter = 10)` or a full criterion such as
    `stopping_criterion = AI.StopAfterIteration(10) | StopWhenConverged(1.0e-10)`.
  * The `message_update_algorithm` kwarg accepts either a NamedTuple of
    keyword arguments forwarded to `SimpleMessageUpdateAlgorithm`, or a
    full `AI.Algorithm`.
  * The `edges` kwarg defaults to `default_beliefpropagation_edges(factors)`
    rather than being computed inside the body.
  * Per-iteration kwarg broadcasting (`extend_columns`, `rows`,
    `rowlength`, `repeat_last`) is removed.

Internals:

  * `BeliefPropagationSweepAlgorithm` now holds a single
    `message_update_algorithm` (was a `Dict{Edge, ...}` of per-edge
    algorithms). Edge-dependent updates are expressed by defining a new
    `AI.Algorithm` subtype and dispatching on `problem.edge` in
    `solve_loop!`.
  * `BeliefPropagationAlgorithm` holds a single `sweep_algorithm` (was a
    `Vector` of per-iteration sweep algorithms). Iteration-dependent
    sweep behavior is expressed by defining a new sweep algorithm
    subtype and varying the inner algorithm in
    `AIE.initialize_subsolve` using `state.iteration`.
  * New `select_*` selectors `select_beliefpropagation_stopping_criterion`
    and `select_message_update_algorithm`, modeled on
    `MatrixAlgebraKit.select_truncation`, normalize NamedTuple-or-object
    inputs into algorithm/criterion objects.
  * `default_beliefpropagation_edges` and
    `default_message_update_algorithm` expose the defaults as named
    functions.
  * Stopping-criterion default removal: the entry point no longer
    constructs a `StopAfterIteration(maxiter)` itself — choice of
    criterion is entirely the caller's responsibility, except for the
    inner sweep, which still defaults to `StopAfterIteration(length(edges))`.
  * Type parameter `SCState` renamed to `StoppingCriterionState` on both
    BP state structs.
  * Helper definitions reordered so default/select helpers sit above
    `beliefpropagation` for readability.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reorder the body of `beliefpropagation` so the algorithm construction
flows top-to-bottom and `cache` is constructed alongside the
`AI.solve` call, and collapse a few short multi-line forms onto single
lines where the formatter is happy with it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…m` strategy

The third `AlgorithmsInterface` layer (`MessageUpdateProblem` /
`SimpleMessageUpdateAlgorithm` / `MessageUpdateState`) was a non-iterative
function call dressed in problem/algorithm/state ceremony; this commit
drops that framing. The per-edge update body lives in
`AI.step!(::BeliefPropagationSweepProblem, ::BeliefPropagationSweepAlgorithm,
::BeliefPropagationSweepState)` and delegates to a new strategy interface:

  - `abstract type MessageUpdateAlgorithm end`
  - `message_update!(algorithm, cache, factors, edge)`
  - `SimpleMessageUpdate <: MessageUpdateAlgorithm` (the default;
    holds `normalize` + `contraction_alg`)

`BeliefPropagationSweepAlgorithm` now holds a `message_update_algorithm`
field (default `SimpleMessageUpdate()`) and is no longer a
`NestedAlgorithm`.

Algorithm selection follows `MatrixAlgebraKit.select_algorithm`. Generic
`default_algorithm(f; kwargs...)` and `select_algorithm(f, alg; kwargs...)`
helpers live in `AlgorithmsInterfaceExtensions`; operations register a
default by overloading `default_algorithm(::typeof(f); kwargs...)`. BP
overloads both for `message_update!`. Callers can pass either an explicit
`MessageUpdateAlgorithm` instance, a `NamedTuple` of keyword arguments
forwarded to the default algorithm, or `nothing` plus flat kwargs (also
forwarded to the default). A convenience signature
`message_update!(cache, factors, edge; alg = nothing, kwargs...)` routes
through `AIE.select_algorithm` so the same dispatch pattern is reachable
from a free-standing call. The `message_update_algorithm` kwarg on
`beliefpropagation` uses the same selector.

To keep the message-store iterate persistent across outer-loop iterations
without duplicating it on the outer state, introduce an `AIE.NestedState`
abstract type with generic `getproperty` / `setproperty!` /
`propertynames` forwarders for `:iterate` through a `:substate` field.
`BeliefPropagationState <: AIE.NestedState` now wraps a
`BeliefPropagationSweepState` as `substate`; the outer state holds no
`iterate` field of its own. The default `AIE.finalize_substate!` (which
copies `substate.iterate` back to `state.iterate`) becomes a self-write
through the forwarder — harmless and removes the need for a BP-specific
override.

Rename: `BeliefPropagationAlgorithm.sweep_algorithm` field →
`subalgorithm` (matches the generic name already used in
`AIE.initialize_subsolve`'s return tuple).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Refine the MAK-style algorithm selector introduced in the previous commit:

* `select_algorithm` / `default_algorithm` now take an `args` tuple as
  their final positional argument (after `alg`). A generic value→type
  wrapper forwards `(::Tuple)` to `(::Type{<:Tuple})`, so operations
  dispatch on the input-tuple type. Wrapping in a tuple keeps the value
  and type domains disjoint — `(1.2,)` is unambiguously a value tuple,
  `Tuple{Float64}` is unambiguously the type form. This matters at sites
  like `beliefpropagation` where not every input has a concrete value
  yet (no specific `edge`), and the type-form call simply passes
  `edgetype(factors)` in its slot.

* New `AIE.AbstractAlgorithm` supertype carries the
  generic "passthrough an explicit algorithm instance" overload of
  `select_algorithm`, so each operation doesn't have to repeat that
  method. `MessageUpdateAlgorithm <: AIE.AbstractAlgorithm`.

* BP call sites updated: `beliefpropagation` constructs the cache earlier
  and uses `Tuple{typeof(cache), typeof(factors), edgetype(factors)}` as
  the args type. The per-call `message_update!(cache, factors, edge; ...)`
  uses the value tuple `(cache, factors, edge)`. Both describe the same
  call shape.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…_criterion

`message_update!` now follows the convention of mutating-functions returning
the object they modify (the message cache), so callers can chain it.

`select_beliefpropagation_stopping_criterion` accepts a `tol` keyword in
the NamedTuple form alongside the existing `maxiter`. Either or both may be
specified; when both are given they are combined with `|` (stop on
whichever fires first). The spin-ice test now uses the NamedTuple form
`stopping_criterion = (; maxiter = 10, tol = 1.0e-10)` to exercise this
path, in place of the explicit `StopAfterIteration | StopWhenConverged`
construction.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@mtfishman mtfishman enabled auto-merge (squash) May 20, 2026 22:06
@mtfishman mtfishman merged commit 27798b7 into main May 20, 2026
17 of 18 checks passed
@mtfishman mtfishman deleted the mf/nested-algorithm branch May 20, 2026 22:11
mtfishman added a commit that referenced this pull request May 20, 2026
Sync the apply-PR's local `NestedAlgorithm` definition with the rename
landing in #115. Once #115 merges, this local definition will be
removed entirely in favor of `AIE.NestedAlgorithm`.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants